Add commits order index by myieye · Pull Request #64 · sillsdev/harmony

myieye · 2026-05-13T10:32:52Z

Adds a composite index on Commits (DateTime, Counter, Id) — the columns of Harmony's canonical order (DefaultOrder), which most commit read paths use (sync, snapshot construction, validation). Without it each of those sorts the full commit set; with it they're index scans, and the win grows with commit count. One all-ASC index covers both directions — DefaultOrderDescending is served by a backward scan.

Low maintenance cost here: commits are append-mostly and the leading (DateTime, Counter) key is ~monotonic, so inserts land at the B-tree tail rather than fragmenting it.

Uses EFCore.ComplexIndexes because EF can't natively index a mix of ComplexProperty (HybridDateTime) members and scalars (dotnet/efcore#11336, targeted for EF 11).

Summary by CodeRabbit

New Features
- Added a composite database index for commit records to improve query performance and retrieval of historical entries.
Chores
- Added support library for advanced indexing and recorded its version in centralized package management.

coderabbitai · 2026-05-13T10:32:59Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c16fed36-80f5-4464-8588-9cedc891ee10

📥 Commits

Reviewing files that changed from the base of the PR and between 263927c and bbf37e2.

📒 Files selected for processing (2)

Directory.Packages.props
src/SIL.Harmony/Db/EntityConfig/CommitEntityConfig.cs

🚧 Files skipped from review as they are similar to previous changes (1)

src/SIL.Harmony/Db/EntityConfig/CommitEntityConfig.cs

📝 Walkthrough

Walkthrough

Adds EFCore.ComplexIndexes (managed version 3.1.5 and project PackageReference) and uses it in CommitEntityConfig to create a named composite index IX_Commits_DateTime_Counter_Id over HybridDateTime.DateTime, HybridDateTime.Counter, and Id; verified model output updated.

Changes

Complex Composite Index Implementation

Layer / File(s)	Summary
EFCore.ComplexIndexes Dependency Setup `Directory.Packages.props`, `src/SIL.Harmony/SIL.Harmony.csproj`	Central `PackageVersion` (`EFCore.ComplexIndexes` = `3.1.5`) added and project `PackageReference` added.
Commit Entity Complex Index Configuration and Verification `src/SIL.Harmony/Db/EntityConfig/CommitEntityConfig.cs`, `src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt`	Added `using EFCore.ComplexIndexes;` and `builder.HasComplexCompositeIndex(...)` to create named index `IX_Commits_DateTime_Counter_Id` over `HybridDateTime.DateTime`, `HybridDateTime.Counter`, and `Id`. Verified model output shows the new index.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through props and csproj lines,
Tucked version three point one point five in fine,
Brought DateTime, Counter, Id in a row,
A named index now helps queries go,
The model hums; the tests say, "Bravo!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add commits order index' directly and clearly summarizes the main change: adding a composite database index on the Commits table to optimize query ordering.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch add-commits-order-index

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/SIL.Harmony/Db/EntityConfig/CommitEntityConfig.cs`:
- Around line 29-31: The call in CommitEntityConfig currently uses a
non-existent named parameter indexName on HasComplexCompositeIndex; switch to
the overload that accepts an Action<ComplexIndexBuilder> and set the
ComplexIndexAnnotations.IndexName inside that configure callback so the index
name "IX_Commits_DateTime_Counter_Id" is applied (i.e., replace the current
HasComplexCompositeIndex(c => new { c.HybridDateTime.DateTime,
c.HybridDateTime.Counter, c.Id }, indexName: "...") with
HasComplexCompositeIndex(columns => ..., cfg =>
cfg.Annotations[ComplexIndexAnnotations.IndexName] =
"IX_Commits_DateTime_Counter_Id" or equivalent on ComplexIndexBuilder).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e6ddd030-9a6c-43d1-a603-143b43aa54d8

📥 Commits

Reviewing files that changed from the base of the PR and between 31dc78b and d1aa44c.

📒 Files selected for processing (4)

Directory.Packages.props
src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt
src/SIL.Harmony/Db/EntityConfig/CommitEntityConfig.cs
src/SIL.Harmony/SIL.Harmony.csproj

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CaffeinatedCoder · 2026-06-04T21:35:40Z

@myieye Hey, I am the author of EFCore.ComplexIndexes. I am very happy that my package is actually being considered useful in other projects as well :-) Wanted to inform you: I just released version 3.1.0 that allows specifying specific sort orders in multi-column complex indexes:

Wrap any member in DbOrder.Desc(...) (or DbOrder.Asc(...), the default) to control its sort order. Because a wrapped member is a method call, C# requires you to name it in the anonymous type:

builder.HasComplexCompositeIndex(
    c => new { c.HybridDateTime.DateTime, Counter = DbOrder.Desc(c.HybridDateTime.Counter), c.Id },
    indexName: "IX_Commits_DateTime_Counter_Id");
// CREATE INDEX "IX_Commits_DateTime_Counter_Id" ON ... ("DateTime", "Counter" DESC, "Id");

3.1.0 added per-column sort direction support, so update the comment to reflect that we don't need it (both Harmony sorts are uniform-direction) rather than that the package can't express it. Co-authored-by: Claude <noreply@anthropic.com>

myieye force-pushed the add-commits-order-index branch from 2e408a0 to 64ea174 Compare May 28, 2026 09:10

Add commits order index

d1aa44c

myieye force-pushed the add-commits-order-index branch from 64ea174 to d1aa44c Compare May 28, 2026 09:53

myieye marked this pull request as ready for review June 2, 2026 12:50

coderabbitai Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread src/SIL.Harmony/Db/EntityConfig/CommitEntityConfig.cs

Bump EFCore.ComplexIndexes to 2.0.5

263927c

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

myieye requested a review from rmunn June 3, 2026 11:50

Bump EFCore.ComplexIndexes to 3.1.5 (#69)

bbf37e2

3.1.0 added per-column sort direction support, so update the comment to reflect that we don't need it (both Harmony sorts are uniform-direction) rather than that the package can't express it. Co-authored-by: Claude <noreply@anthropic.com>

hahn-kev approved these changes Jun 11, 2026

View reviewed changes

myieye merged commit 913e018 into main Jun 11, 2026
6 checks passed

myieye deleted the add-commits-order-index branch June 11, 2026 09:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add commits order index#64

Add commits order index#64
myieye merged 3 commits into
mainfrom
add-commits-order-index

myieye commented May 13, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 13, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

CaffeinatedCoder commented Jun 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

myieye commented May 13, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CaffeinatedCoder commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

myieye commented May 13, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 13, 2026 •

edited

Loading

CaffeinatedCoder commented Jun 4, 2026 •

edited

Loading